-
Notifications
You must be signed in to change notification settings - Fork 203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: change isDepositMode to getTransferMode #2031
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
packages/arb-token-bridge-ui/src/util/__tests__/isDepositMode.test.ts
Outdated
Show resolved
Hide resolved
packages/arb-token-bridge-ui/src/util/__tests__/isDepositMode.test.ts
Outdated
Show resolved
Hide resolved
it('should return true for L2 source chain and L3 destination chain', () => { | ||
const result1 = isDepositMode({ | ||
sourceChainId: ChainId.ArbitrumOne, | ||
destinationChainId: 660279 // Xai |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not related to this PR, but it would be nice to have enum for chain ids somewhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps we can generate enums from the orbit chains data json, but i think the original idea was to only keep core chain's ids as enum
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for this test at least, we could move the definition of the chain ids to the outer scope so we don't need to add comments every time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused that isDepositMode
returns true
for teleports, so I think we should talk first about what we consider deposits and figure out cleanups
b2c7fb4
Closes FS-929